Switch to the rust component graph #1295
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shsms
requested review from
Copilot and
daniel-zullo-frequenz
and removed request for
a team
github-actions
bot
added
part:docs
Contributor
Author
|
Based on #1283 |
There was a problem hiding this comment.
Pull Request Overview
This PR switches from a Python-based component graph and formula engine to a new Rust-based implementation, introducing new Formula and Formula3Phase classes to support advanced formula functions (max/min/coalesce) and removing deprecated code.
Key Changes:
- Replaced old Python formula engine with new Rust-based component graph
- Implemented new
FormulaandFormula3Phaseclasses with support for max/min/coalesce functions - Updated component data structures to use new client library types
- Removed deprecated formula engine tests and implementation
Reviewed Changes
Copilot reviewed 109 out of 118 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/timeseries/test_formula_engine.py | Entire file deleted - old formula engine tests removed |
| tests/timeseries/test_consumer.py | Added missing resampler calls for battery and PV inverter power |
| tests/timeseries/mock_resampler.py | Updated to use new Metric enum instead of ComponentMetricId |
| tests/timeseries/mock_microgrid.py | Updated component types and connection structures to match new API |
| tests/timeseries/_formulas/test_lexer.py | New test file for formula lexer functionality |
| tests/timeseries/_formulas/test_formulas_3_phase.py | New test file for three-phase formula support |
| tests/timeseries/_formulas/test_formulas.py | New comprehensive formula tests with max/min/coalesce support |
| tests/timeseries/_formulas/test_formula_composition.py | Updated to use new formula API and async context managers |
| src/frequenz/sdk/timeseries/formulas/_token.py | New token definitions for formula parsing |
| src/frequenz/sdk/timeseries/formulas/_resampled_stream_fetcher.py | New component for fetching resampled telemetry streams |
| src/frequenz/sdk/timeseries/formulas/_peekable.py | New peekable iterator implementation for parsing |
| src/frequenz/sdk/timeseries/formulas/_parser.py | New formula parser implementation |
| src/frequenz/sdk/timeseries/formulas/_lexer.py | New formula lexer implementation |
| src/frequenz/sdk/timeseries/formulas/_functions.py | New function implementations (max/min/coalesce) |
Comments suppressed due to low confidence (1)
tests/timeseries/mock_microgrid.py:9
- The import
Callablefromcollections.abcon line 10 is unused after the changes. TheCallabletype is imported again on line 12 from thetypingmodule via theCoroutineimport, and all usages in the file appear to use the typing module'sCallable(e.g., line 260). This unused import should be removed.
from collections.abc import Callable
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/microgrid/power_distributing/_component_status/test_battery_status.py
Show resolved
Hide resolved
shsms
force-pushed
the
new-formulas+new-graph=
branch
from
4583c7d to
c942e5f
Compare
Contributor
|
Can you rebase it on top of Based on #1293 instead? |
Contributor
Author
Contributor
Author
|
Oh, maybe those changes are from the api PR. don't know. Let's take the API PR first then, we can decide. |
shsms
force-pushed
the
new-formulas+new-graph=
branch
2 times, most recently
from
706118b to
cc1e61b
Compare
Contributor
Author
|
It was actually trivial to rebase on top of #1293. Done. |
shsms
force-pushed
the
new-formulas+new-graph=
branch
from
cc1e61b to
09b029b
Compare
Contributor
Author
|
Damn, this causes my tests to fail. Switched back to based on #1283. |
Signed-off-by: Leandro Lucarella <[email protected]>
Since we are taking several `timedelta`s, it could be a bit confusing on the call site to know which is which, so better to require to use them as keyword arguments. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
After some refactoring, result can't be `None` anymore, so we don't need to consider that case. Signed-off-by: Leandro Lucarella <[email protected]>
Import from `collections.abc` instead, and also use a direct import of `cast` instead of using it from `typing`. Signed-off-by: Leandro Lucarella <[email protected]>
The _Mocks class already provides a `.stop()` methods to cleanup, so use that to avoid duplicated code. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
This is useful when debugging tests, to get a nice representation of the component graph. Signed-off-by: Leandro Lucarella <[email protected]>
Type stubs are now [officially available][1], so we use them instead of instruct `mypy` to ignore type hints for the `networkx` module. [1]: networkx/networkx#3988 (comment) Signed-off-by: Leandro Lucarella <[email protected]>
This makes the terminology more precise, avoids confusion, and matches the new microgrid API naming. Signed-off-by: Leandro Lucarella <[email protected]>
Use the URL for the v0.18 sandbox. This is a WIP because it actually points to the v0.17 sandbox, as there is no v0.18 sandbox yet. Signed-off-by: Leandro Lucarella <[email protected]>
shsms
force-pushed
the
new-formulas+new-graph=
branch
2 times, most recently
from
49e8ff8 to
c88d813
Compare
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
It sends requests to the resampler and fetches resampled telemetry streams. Signed-off-by: Sahas Subramanian <[email protected]>
Waits for new values from the input data streams. When there's one new value from each of them, evaluates the AST and sends the calculated value out. Signed-off-by: Sahas Subramanian <[email protected]>
This is a wrapper around the FormulaEvaluatingActor, with methods for composing multiple formulas. Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
This also adds an evaluator and tests for the 3-phase formulas. Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
This will replace the SDK's old component graph and the formula generators. This commit also adds some graph traversal methods that are not provided by the component graph library yet. Signed-off-by: Sahas Subramanian <[email protected]>
Graphs in island mode are not supported yet. Earlier, this test was using an `Unspecified` component category as a junction node, to start traversing from, which is inaccurate. The `Unspecified` component should always be an error, to identify an unfilled category field, due to a bug. Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
This won't be necessary as soon as the coalesce AST node is able to subscribe to secondary streams only if primary streams are not available. But for now, coalesce nodes need all their input streams to have data. Signed-off-by: Sahas Subramanian <[email protected]>
The new formulas use component metrics as primary and only when those are not available, uses any corresponding meters as fallback. Signed-off-by: Sahas Subramanian <[email protected]>
The new formulas use the CHP meter in the grid formula, and in the CHP formula, both of which are in different namespaces. This change makes the same data available from two separate streams from the mock_resampler. Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
shsms
force-pushed
the
new-formulas+new-graph=
branch
from
c88d813 to
4f25d12
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Formula/Formula3Phaseclasses to replace the old formula engine, for supporting the new formula functions - max/min/coalesce.